Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Hubble Stats code #1481

Merged
merged 10 commits into from
Aug 2, 2023
Merged

Refactor Hubble Stats code #1481

merged 10 commits into from
Aug 2, 2023

Conversation

vutuanlinh2k2
Copy link
Contributor

Summary of changes

Provide a detailed description of proposed changes.

  • Add favicon
  • Update to utilize typedChainId instead of chain names for Shielded tables and Filter Button
  • Optimize 'use client' usage
  • Refactor existing components

Proposed area of change

Put an x in the boxes that apply.

  • apps/bridge-dapp
  • apps/hubble-stats
  • apps/stats-dapp
  • apps/webbsite
  • apps/faucet
  • apps/tangle-website
  • libs/webb-ui-components

Reference issue to close (if applicable)

Specify any issues that can be closed from these changes (e.g. Closes #233).


Code Checklist

Please be sure to add .stories documentation if any additions are made to libs/webb-ui-components.

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit b7553ce0cec04b79d3eafdaa8e985eae1a503368
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64c8bcb0986e8d512cdf0ccd
😎 Deploy Preview https://64c8bcb0986e8d512cdf0ccd--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@vutuanlinh2k2 vutuanlinh2k2 added the needs review 👓 Pull request needs a review label Aug 1, 2023
Copy link
Member

@AtelyPham AtelyPham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that you've refactored some components in the hubble-stats dapp to be client components. While your effort is much appreciated, I'm not entirely convinced about the benefits of these changes. Could you kindly take another look at these components? Just as a reminder, client components are those that manage the client's state or side effects. Thanks in advance for your consideration!

apps/hubble-stats/components/ShieldedPoolsTable/types.ts Outdated Show resolved Hide resolved
apps/hubble-stats/components/Breadcrumbs/Breadcrumbs.tsx Outdated Show resolved Hide resolved
Comment on lines +1 to +2
'use client';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay, but I propose we create a task to review our components in the webb-ui-kit. The aim would be to determine which components are client-related, and which are associated with the server. Could you kindly create an issue to address this?

@AtelyPham AtelyPham added needs changes 🔧 PR has requested changes or conflicts that need to be addressed and removed needs review 👓 Pull request needs a review labels Aug 1, 2023
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit 0af328e0b306ca7233f3842d4aae84cf23889c8e
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64ca2216d6bea1007211f629
😎 Deploy Preview https://64ca2216d6bea1007211f629--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@vutuanlinh2k2 vutuanlinh2k2 added needs review 👓 Pull request needs a review and removed needs changes 🔧 PR has requested changes or conflicts that need to be addressed labels Aug 2, 2023
@AtelyPham AtelyPham merged commit 76a845c into develop Aug 2, 2023
7 checks passed
@AtelyPham AtelyPham deleted the linh/hubble-stats-refactor branch August 2, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review 👓 Pull request needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK] Refactor code for Hubble Stats [TASK] Remove options with Edgeware and use regular options
2 participants